fix(lwdid): preserve fitted overall inference - #1
Conversation
Maintainer rebase onto current main (igerber, 2026-07-17), per plan agreed in PR igerber#588: dropped committed datasets (tutorial now uses the checksummed load_prop99()/load_walmart() loaders on main), kept the maintainer-authored paper reviews and references entries from igerber#685, removed the lwdid dev dependency (external reference implementations stay environmental, importorskip-gated), renumbered tutorial 26 -> 27. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Fix IPW SE centering (translation-invariant influence function) - Implement composite regression (Eq 7.18/7.19) for staggered ATT - Add N_infinity >= 2 guard for never-treated controls - Implement event study (Appendix D): WATT(r) + Algorithm 1 sup-t bands - Fix randomization inference p-value convention (strict > comparison) - Add design validation (absorbing treatment, time-invariant cohort) - Migrate custom exceptions to ValueError - Trim top-level exports to LWDiD/LWDiDResults/LW Acceptance: 39 pass / 9 xfail (IPWRA event-study variants pending). Remaining xfails are IPWRA-specific event-study cases for Step 3.
- Execute all 30 code cells with full outputs (text + plots) - Add missing top-level exports to __init__.py: randomization_inference, wild_cluster_bootstrap, test_parallel_trends, sensitivity_analysis, recommend_transformation
…puts - Fix IPWRA event-study golden tests (pass controls in test helper) - Resolve mypy type errors to zero (type: ignore for pandas Union types) - Execute tutorial notebook with outputs (30/30 cells) - Fix tutorial imports to use module-level paths (per export trim) - Remove stale XFAIL_IPW_CENTERING marker Methodology tests: 43 pass / 5 xfail (non-strict bootstrap SE only). All strict acceptance criteria met.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
c2694ff to
10c900a
Compare
…te contract Four groups, one per issue in this round. TestCohortTimeCellSupport pins the (g,t) construction (igerber#734): eligibility is period-specific rather than per-cohort and matches never-treated plus {G > max(g,t)} for every cell; a pure common time trend gives ATT = 0; a constant shift applied to every unit-period leaves the ATT unchanged; unsupported cells are reported with a reason and a cohort with no supported cell is dropped. TestInfluenceFunctionReconciliation checks that the influence function each estimator returns actually reproduces that estimator's reported SE, with and without covariates, and that psm reports no influence function instead of a silently independent one. TestStaggeredJointInference covers the overall SE (igerber#735): the joint basis is reported, the SE is wider than the cohort-independence formula it replaces, and it agrees with a unit-cluster bootstrap. TestAggregationContract covers igerber#733 and igerber#732: aggregate("simple") reproduces fit() exactly including df_inference and across every inference basis, "group" carries cohort effects on the shared schema, "event_study" returns EventStudyResults with the anchor as a reference row and survives to_dict(), and the rejected cases (unknown type, weights selector, balance_e off event_study, common-timing fits) all fail closed. test_simple_preserves_the_fitted_result and test_matches_unit_cluster_bootstrap are adapted from Charles Shaw's patch in #1, which independently identified the aggregate() SE discrepancy behind igerber#733. Co-authored-by: Charles Shaw <shawcharles@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for this, @shawcharles — and for filing igerber#732, igerber#733, igerber#734 and igerber#735. The I should explain why I am not merging this patch directly. Addressing igerber#734 required replacing the staggered estimation path wholesale rather than patching it. The old code applied control eligibility as a unit-level filter and then averaged unequal calendar windows across cohorts, which is what let a pure common time trend come through as ATT ≈ 1.0 with SE ≈ 5e-16. Estimation now runs one cell per The behaviour you were asserting is in place, and both of your tests came across with it:
Both are in commit So this PR can be closed as superseded — I'll leave that to you rather than closing it myself. Thanks again for the review; the four issues materially improved the estimator. |
|
Thanks for the detailed explanation, and for carrying the two behavioural tests forward into Since the I will keep review attention on igerber#588 and the remaining CI/review surface there. |
3 participants
Summary
Implements the bounded result-provenance fix in igerber#733.
LWDiD.fit()already computes and stores the canonical overall ATT and its inference.LWDiDResults.aggregate(\"overall\")previously replaced that result with a separate aggregation built from marginal cohort effects and their standard errors. That could change the point estimate and discarded the composite-regression joint inference where it was available.This change returns an equivalent result object containing the fitted canonical result instead of reconstructing a second estimator.
This does not establish the validity of the non-composite delta-method variance used by
fit()for other staggered paths. That path still omits cross-cohort covariance; this PR ensuresaggregate()no longer hides its provenance by replacing it with a different calculation. The remaining statistical work is tracked in igerber#735.Test
df_inferencebetweenfit()andaggregate(\"overall\").tests/test_lwdid.py: 64 passed.This PR targets the source branch of igerber#588 so it can be merged there directly.